Replace existing sources before updating

Currently sources may acquire file locks to ensure that they're not tampered
with while they're in use. We may load two sources to the same location,
however, in the case of git repositories which need to be updated. Cargo will
first load a locked version of the source and then may load an unlocked version,
and these two loads currently deadlock.

This commit tweaks the logic when updating a source to only update it after the
previous source has been replaced.

Closes #2533

diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs
index c005703860ae557094cf576a9398d69c1ae28b2e..153a734d41c0142a52d8cfccdeefaa58e2a19972 100644 (file)
--- a/src/cargo/core/registry.rs
+++ b/src/cargo/core/registry.rs
@@ -158,21 +158,16 @@ impl<'cfg> PackageRegistry<'cfg> {
 
     fn load(&mut self, source_id: &SourceId, kind: Kind) -> CargoResult<()> {
         (|| {
-            let mut source = source_id.load(self.config);
-
-            // Ensure the source has fetched all necessary remote data.
-            let p = profile::start(format!("updating: {}", source_id));
-            try!(source.update());
-            drop(p);
-
+            // Save off the source
+            let source = source_id.load(self.config);
             if kind == Kind::Override {
                 self.overrides.push(source_id.clone());
             }
-
-            // Save off the source
             self.add_source(source_id, source, kind);
 
-            Ok(())
+            // Ensure the source has fetched all necessary remote data.
+            let _p = profile::start(format!("updating: {}", source_id));
+            self.sources.get_mut(source_id).unwrap().update()
         }).chain_error(|| human(format!("Unable to update {}", source_id)))
     }
 

diff --git a/tests/test_cargo_compile_git_deps.rs b/tests/test_cargo_compile_git_deps.rs
index 4e0ca48710edaa30f942b2813f800ce12e6643c3..052db3820953b7623db9b6b93aed7105de6d3268 100644 (file)
--- a/tests/test_cargo_compile_git_deps.rs
+++ b/tests/test_cargo_compile_git_deps.rs
@@ -1749,3 +1749,49 @@ test!(denied_lints_are_allowed {
 {compiling} foo v0.0.1 ([..])
 ", compiling = COMPILING, updating = UPDATING)));
 });
+
+test!(add_a_git_dep {
+    let git = git::new("git", |p| {
+        p.file("Cargo.toml", r#"
+            [project]
+            name = "git"
+            version = "0.5.0"
+            authors = []
+        "#)
+        .file("src/lib.rs", "")
+    }).unwrap();
+
+    let p = project("foo")
+        .file("Cargo.toml", &format!(r#"
+            [package]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+
+            [dependencies]
+            a = {{ path = 'a' }}
+            git = {{ git = '{}' }}
+        "#, git.url()))
+        .file("src/lib.rs", "")
+        .file("a/Cargo.toml", r#"
+            [package]
+            name = "a"
+            version = "0.0.1"
+            authors = []
+        "#)
+        .file("a/src/lib.rs", "");
+
+    assert_that(p.cargo_process("build"), execs().with_status(0));
+
+    File::create(p.root().join("a/Cargo.toml")).unwrap().write_all(format!(r#"
+        [package]
+        name = "a"
+        version = "0.0.1"
+        authors = []
+
+        [dependencies]
+        git = {{ git = '{}' }}
+    "#, git.url()).as_bytes()).unwrap();
+
+    assert_that(p.cargo("build"), execs().with_status(0));
+});